EMD Velox: update error msg for missing sparse module #305
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #305 +/- ##
==========================================
- Coverage 87.67% 87.66% -0.02%
==========================================
Files 83 83
Lines 11149 11152 +3
Branches 2414 2414
==========================================
+ Hits 9775 9776 +1
- Misses 860 862 +2
Partials 514 514 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Thank you @dbdurham for the pull request, this looks good. Can you try to add a test in https://github.com/hyperspy/rosettasciio/blob/main/rsciio/tests/test_emd_velox.py to cover the code that you added? It will need to catch the error using pytest.raises but also update the pytest.importorskip("sparse") accordingly to enable running the test.
I am happy to do that if this helps.
Can you add a changelog entry? The instruction on how to do that are in https://github.com/hyperspy/rosettasciio/blob/main/upcoming_changes/README.rst.
|
@ericpre sure I will try that, thanks for your suggestions as this is my
first time contributing to a large project like this. May take me a few
days to get to the tests and get it right if that's ok, as it's also my
first time working with pytest.
…On Wed, Aug 28, 2024 at 1:18 PM Eric Prestat ***@***.***> wrote:
***@***.**** commented on this pull request.
Thank you @dbdurham <https://github.com/dbdurham> for the pull request,
this looks good. Can you try to add a test in
https://github.com/hyperspy/rosettasciio/blob/main/rsciio/tests/test_emd_velox.py
to cover the code that you added? It will need to catch the error using
pytest.raises but also update the pytest.importorskip("sparse")
accordingly to enable running the test.
I am happy to do that if this helps.
Can you add a changelog entry? The instruction on how to do that are in
https://github.com/hyperspy/rosettasciio/blob/main/upcoming_changes/README.rst
.
—
Reply to this email directly, view it on GitHub
<#305 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AMSHAAHWFCMNEDLT2IGMVY3ZTYH7LAVCNFSM6AAAAABNEVMG5CVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDENRWHE3TCMBWG4>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Description of the change
This is an update in response to Issue #296 "emd reader Velox ModuleNotFoundError." When attempting to load EDS Spectrum Image data from a Velox EMD file without the optional "sparse" dependency installed, a ModuleNotFoundError is raised which is confusing to some users. To clarify the issue, a try except statement was added around running the Velox EMD file reader and a new error message is printed.
Progress of the PR
upcoming_changesfolder (seeupcoming_changes/README.rst),docs/readthedocs.org:rosettasciiobuild of this PR (link in github checks)